feat(assets-controller): add getAsset#9521
Conversation
Kriys94
force-pushed
the
feat/WPN-1473-assets-controller-get-asset
branch
3 times, most recently
from
8bdabbe to
6aa280d
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6aa280d. Configure here.
| [{ id: accountId }], | ||
| [chainId], | ||
| ['fungible'], | ||
| ); |
There was a problem hiding this comment.
Skips enabled-chain filtering
Medium Severity
getAsset passes only the asset’s own chain into #getAssetsFromState, while default getAssets filters with #enabledChains. A balance on a disabled network can be returned by getAsset but omitted from getAssets, breaking the documented parity between the two APIs.
Reviewed by Cursor Bugbot for commit 6aa280d. Configure here.
There was a problem hiding this comment.
Well this function will be used by Snaps independently from the enabledChains
Co-authored-by: Cursor <cursoragent@cursor.com>
Kriys94
force-pushed
the
feat/WPN-1473-assets-controller-get-asset
branch
from
6aa280d to
2ccbbe5
Compare
| const assets = this.#getAssetsFromState( | ||
| [{ id: accountId }], | ||
| [chainId], | ||
| ['fungible'], | ||
| ); |
There was a problem hiding this comment.
nit - A tad nuts that we are calculating all assets, only to return a single asset here.
This is okay for now, we can raise/improve moving forward.
|
|
||
| #getAssetsFromState( | ||
| accounts: InternalAccount[], | ||
| accounts: Pick<InternalAccount, 'id'>[], |
There was a problem hiding this comment.
Very good! Keep interface minimal!
Easier to test and use.
| }); | ||
| }); | ||
|
|
||
| describe('getAsset', () => { |
There was a problem hiding this comment.
🌶️ this test file is too big, impossible to read!
Thoughts on:
- Making this spec more dry. I think we can leverage test tables (
it.each) - spicer -- separate test for each method, truly isolated, easy to read 😈
There was a problem hiding this comment.
I'm fine keeping this as is... but I'd like to pencil in test cleanup.
Prithpal-Sooriya
left a comment
Prithpal-Sooriya
left a comment
There was a problem hiding this comment.
Approving with comments
2 participants
Explanation
WPN-1472 centralizes the Assets domain in
@metamask/assets-controllerso consumers (including Snaps) can read controller-owned balances, metadata, prices, and fiat values instead of tracking them independently. Today the controller exposesgetAssets(bulk, per-account map),getAssetMetadata(metadata only), andgetAssetsPrice, but there is no way to read a single combined asset for a specific account/asset pair.This PR adds a
getAssetread API toAssetsController:getAsset(accountId, assetId): Asset | undefinedreturns the combinedAsset(balance+metadata+price+ computedfiatValue) for a single account/asset pair from current controller state, orundefinedwhen a complete renderable asset is not available.AssetsController:getAssetmessenger action so it can be called through the messenger (e.g. by preinstalled Snaps once WPC-1017 lands).Behavior details:
getAssets(via the private#getAssetsFromState) so the returned shape never drifts:balanceandmetadataare required; a missing price falls back to{ price: 0, lastUpdated: 0 }withfiatValue: 0; hidden/filtered assets resolve toundefined.undefined) on an emptyaccountIdor a malformed CAIP-19assetId, so consumers can distinguish bad requests from absent data.assetId(checksums EVM ERC-20 addresses) before lookup, consistent with how state is keyed.#getAssetsFromState'saccountsparameter was widened toPick<InternalAccount, 'id'>[](it only readsaccount.id) sogetAssetcan reuse it without constructing a full account object.The generated messenger action types (
AssetsControllerGetAssetAction) were regenerated and exported fromindex.ts, and a### Addedchangelog entry was added.References
getAssetread API for Snap consumersChecklist
Note
Low Risk
Additive read-only API with tests and aligned with existing
getAssetscomposition; no fetch or state-write behavior changes beyond the new method signature tweak on#getAssetsFromState.Overview
Adds
getAsset(accountId, assetId)and theAssetsController:getAssetmessenger action so consumers can read one fully composedAsset(balance, metadata, price,fiatValue) from current state without calling bulkgetAssets.Implementation routes through
#getAssetsFromStatewith the same filtering asgetAssets(requires balance + metadata, zero price fallback, hidden assets excluded), normalizes CAIP-19 IDs, validates inputs (throws on empty account or invalid asset id), and does not trigger fetches.#getAssetsFromStatenow acceptsPick<InternalAccount, 'id'>[]so single-asset lookups do not need full account objects.Exports
AssetsControllerGetAssetAction, registers/unregisters the messenger handler, documents the API in generated action types, and adds changelog + unit tests.Reviewed by Cursor Bugbot for commit 2ccbbe5. Bugbot is set up for automated code reviews on this repo. Configure here.